Skip to content
This repository has been archived by the owner on Sep 2, 2021. It is now read-only.

Support initialDirectory and title in Linux' showOpenDialog #496

Merged
merged 2 commits into from
Jan 8, 2015

Conversation

marcelgerber
Copy link
Contributor

For adobe/brackets#10297
I weren't able to build this due to various errors (in a Ubuntu 14.10 VM), not even on top of the linux-1547 branch.

I also don't know what to do with fileTypes - it currently doesn't work either.

@ingorichter
Copy link
Contributor

I'm going to check this on my 14.10 VM

@ingorichter
Copy link
Contributor

I was able to compile and run this on my 14.10 VM (32 Bit). Looks good so far.
Please try to delete the out folder and rebuild with grunt build. That helped on my machine.
We can use the fileTypes parameter and set the filter with http://www.geany.org/manual/gtk/gtk/GtkFileChooser.html#gtk-file-chooser-set-filter. This sounds like a good use of it.

@marcelgerber
Copy link
Contributor Author

I still wasn't able to compile this on my 14.10 VM (32 Bit), even though I ran git clean and started the setup from ground up.

I'm also not too experienced with native code and that's why I honestly don't know how to implement fileTypes. It looks like right now, it's not even implemented on Windows, so the param is quite useless anyway. Also, I don't know how an array on the JavaScript site can be converted to an ExtensionString on the native site.

@ingorichter As you managed building it, could you just make sure that running showOpenDialog with an undefined title still shows some default title?

@ingorichter
Copy link
Contributor

@marcelgerber let me check tomorrow if we have any occurrences where we pass fileTypes to this function. Otherwise we can omit the implementation. But if this isn't implemented on Windows, I guess that we are not using it anywhere. I think we shouldn't change the function signature, but add a comment to the functions to make clear, that there is currently no need to support this feature. I assume, that the original plan to implement this feature was driven by the observations what the open/save dialogs provide for each platform. Since you can have this file filter functionality on all supported platforms, the engineer who implemented this was preparing for a possible use of it.
I'm going to check the behavior with an undefined title argument.

@@ -164,15 +164,21 @@ int32 ShowOpenDialog(bool allowMultipleSelection,
CefRefPtr<CefListValue>& selectedFiles)
{
GtkWidget *dialog;
const char* dialog_title = chooseDirectory ? "Open Directory" : "Open File";
// const char* dialog_title = chooseDirectory ? "Open Directory" : "Open File";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can get rid of this line, since the caller provides the specific title.

@ingorichter
Copy link
Contributor

Passing undefined or null for the title results in the default title Open. This is totally fine to move forward. Let's get the one comment resolved and then we can merge it.

@marcelgerber
Copy link
Contributor Author

Done. Ready for merge.

ingorichter added a commit that referenced this pull request Jan 8, 2015
Support initialDirectory and title in Linux' showOpenDialog
@ingorichter ingorichter merged commit 89977a9 into adobe:master Jan 8, 2015
@ingorichter
Copy link
Contributor

Thanks @marcelgerber. Merged.

@peterflynn
Copy link
Member

Fwiw, here's the Trello card for missing fileTypes support on Windows: https://trello.com/c/9dtYWEUj/20-1-windows-open-dialog-should-respect-passed-in-file-type-filters-see-showopendialog-executeshowopendialog. Could be expanded to cover Linux, too...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants